[dv] Fix directed-test configuration and scheduling - #2497
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document. By submitting this pull request comment, I am hereby confirming my acceptance of the terms of the CLA Document and my agreement to be legally bound by its terms. |
ea69428 to
0ae0e8a
Compare
SamuelRiedel
left a comment
There was a problem hiding this comment.
Thanks @kulan-pal and sorry for the delay. This looks good to me. I just have one small nit.
| def list_tests(dir): | ||
| testlist_str = os.popen('ls '+dir).read() | ||
| testlist = [] | ||
| for test in testlist_str.split('\n')[:-1]: | ||
| testlist.append(test) | ||
| print(testlist) | ||
| testlist = sorted(os.listdir(dir)) | ||
| print(testlist) | ||
| return testlist |
There was a problem hiding this comment.
I think this function is never used anywhere.Would you mind just removing it instead of fixing it?
0ae0e8a to
e04906b
Compare
No worries, I understand how busy you are. Thanks for taking a look at my PRs. |
run_rtl.py passes a test's sim_opts to the simulator, but the schema for directed tests has no sim_opts field. pydantic drops unknown keys, so a directed test cannot set plusargs in its testlist entry. Add the field to DConfig as optional, so existing entries stay valid. On DConfig a config can set a default that a test still overrides. Signed-off-by: Kulan Palanichamy <kulan.palanichamy@opentitan.org>
The append that accepts a test sat inside the loop over its rtl_params, so a test with N parameters was scheduled N times. Every current test has one parameter, which hid it. Move the append to the loop's else clause, so it runs once after all parameters have matched. Signed-off-by: Kulan Palanichamy <kulan.palanichamy@opentitan.org>
gen_testlist.py listed the test directories with ls, whose order depends on the locale: sh-misaligned sorts before shamt under C and after it under en_US. The generated directed_testlist.yaml therefore depended on who ran the script. Use sorted(os.listdir()) and regenerate the file. No entry changes, only the order of a few vendored tests. Remove list_tests(), which nothing calls. Signed-off-by: Kulan Palanichamy <kulan.palanichamy@opentitan.org>
e04906b to
41a575d
Compare
hcallahan-lowrisc
left a comment
There was a problem hiding this comment.
This is nice. Thanks a lot @kulan-pal
Three small fixes to the directed-test flow in
dv/uvm/core_ibex. None of them changes which tests run today.They are needed by directed tests that set their own plusargs (#2498, #2500).
sim_optswas dropped.run_rtl.pypassessim_optsto the simulator, but the directed-test schema hadno such field, so pydantic threw it away. It is now an optional field of
DConfig.rtl_paramsran N times. The accept infilter_tests_by_configsat inside the loop over theparameters. It now runs once, in the loop's
else:. Every current test has one parameter, so this never showed.directed_testlist.yamldepended on the locale.gen_testlist.pylisted directories withls; it nowuses
sorted(os.listdir()). The regenerated file has the same 944 entries, with a few vendored tests reordered.Checked on e1a6be2: the testlist regenerates byte-identical, and
zcmp_reserved_testfrom #2498 (twortl_paramsand asim_opts) is scheduled once with its plusarg on the simulator command line.The
directed_testsbranch adds the same field toDTest. Whichever lands second can drop its copy.AI disclosure (CLA §9): written with help from Claude Code and reviewed with OpenAI Codex; I have reviewed and understood every change and take full responsibility for it.